Skip to content

Make sure we're using the new account system everywhere - #999

Merged
astralbodies merged 27 commits into
developfrom
feature/22-accounts
Jan 7, 2014
Merged

Make sure we're using the new account system everywhere#999
astralbodies merged 27 commits into
developfrom
feature/22-accounts

Conversation

@koke

@koke koke commented Jan 3, 2014

Copy link
Copy Markdown
Member

Some parts of the code still use [WordPressComApi sharedApi].username to get the WordPress.com account. Migrate everything to the new WPAccount system.

Fixes #22

koke and others added 25 commits December 5, 2013 18:15
…dPress-iOS into issue/22-remove-comapi-warnings

Conflicts:
	WordPress/Classes/Blog.m
	WordPress/Classes/NotificationsManager.m
	WordPress/Classes/WPMobileStats.m
	WordPress/Classes/WPcomLoginViewController.m
	WordPress/Classes/WordPressAppDelegate.m
…warnings

#22 Remove WordPressComApi deprecation
If this in fact needed, it can be reverted. It was not called.
Note @koke, that for JetpackSettingsVC, the effect of creating a new account may not be the correct approach taken. signInWithUsername:password:blocks use to store the authtoken internally within WordPressComApi. Now, the only way to save a token is by creating a new account.

We could make `refreshTokenWithSuccess:failure` useful again for these kinds of purposes.
- WordPressComOAuthClient should likely handle the keychain for auth token

- Remove kWPcomXMLRPCUrl as self.xmlrpc for a dot com blog is the same

- Collapse various const NSString keys that had the same value
Registers and gets settings via XML RPC API
- Move app usage into NotificationsManager

- Make WordPressComApi app-independent
- Move app usage into NotificationsManager

- Make WordPressComApi app-independent
- Move app usage into NotificationsManager

- Make WordPressComApi app-independent
- Notification was posted for checkForUnseenNotifications

- Also turns out that this method is called, but nothing observes the notification

- Removed the call from the app in NotificationsManager.
- WPToast: present toast from callers instead

- Constants, WPAccount, ContextManager, Note
- Calls to WordPressComApi discovered that relied on the Notes being synced
- Split out the API usage into a category for organization

- Rename confusing ‘sync’ method to `mergeNewNotes:` like other models
- checkNotifications -> fetchRecentNotifications; fetches the last 20
- Remove redundant getter, use property getter instead
- While not ideal, it gives more meaning to the API calls

- Most Notes related calls sync the notes after the response comes back, so keep this in Note

- Remove redundant ‘user’ reference in controller that points to WordPressComApi
…nto feature/22-accounts

Conflicts:
	WordPress/Classes/BlogListViewController.m
	WordPress/Classes/NotificationsFollowDetailViewController.m
	WordPress/Classes/ReaderPost.m
	WordPress/Classes/SettingsViewController.m
	WordPress/Classes/WPAccount.m
Move the default account URL as recipients of the notification fired attempt to use the default account while deletion is happening, even if it's through the same context. Fixes a visual issue where the sign out did not cause the visual status of settings to be correct.
Results controller should take care of noticing when account changes

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unrelated to accounts and I think we're in string freeze now. Should we move this to a separate ticket/branch? @sendhil @xtreme-rebecca-putinski

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koke - we are in a string freeze, but I see both "Unfollowed" and "Followed" are already localized.

@astralbodies

Copy link
Copy Markdown
Contributor

I think I might be conflicting with some of the changes in PR #1000 - I'll review this PR now

Conflicts:
	WordPress/Classes/BlogSelectorViewController.m
	WordPress/Classes/Constants.h
	WordPress/Classes/CreateAccountAndBlogViewController.m
	WordPress/Classes/EditSiteViewController.m
	WordPress/Classes/JetpackSettingsViewController.m
	WordPress/Classes/Note.h
	WordPress/Classes/NotificationsCommentDetailViewController.m
	WordPress/Classes/NotificationsManager.m
	WordPress/Classes/NotificationsViewController.m
	WordPress/Classes/ReaderCommentFormView.m
	WordPress/Classes/ReaderPostsViewController.m
	WordPress/Classes/SettingsViewController.m
	WordPress/Classes/WPAccount.m
	WordPress/WordPressApi/WordPressComApi.m
@astralbodies

Copy link
Copy Markdown
Contributor

Overall this looks good - I'm going to wait to merge #1000 until this is in develop and I'll merge develop into there to make sure my APN fixes still apply.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this accidentally excluded?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, collateral damage from a merge?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That constant isn't used anywhere in WPAccount

@astralbodies

Copy link
Copy Markdown
Contributor

So I'm seeing a bunch of compilation errors now that I finally checked out the branch. I'm assuming not intentional 😄.

@koke

koke commented Jan 6, 2014

Copy link
Copy Markdown
Member Author

Code looks good after some merge hiccups, but needs more testing

@koke

koke commented Jan 7, 2014

Copy link
Copy Markdown
Member Author

:shipit:

@astralbodies

Copy link
Copy Markdown
Contributor

Looks good for merging :shipit:

astralbodies added a commit that referenced this pull request Jan 7, 2014
Make sure we're using the new account system everywhere
@astralbodies
astralbodies merged commit 5884ce3 into develop Jan 7, 2014
@astralbodies
astralbodies deleted the feature/22-accounts branch January 7, 2014 18:52
@astralbodies

Copy link
Copy Markdown
Contributor

I'm deleting the branch but we can restore it if needed.

@sendhil

sendhil commented Jan 7, 2014

Copy link
Copy Markdown
Contributor

💥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make sure we're using the new account system everywhere

4 participants